fix: subscriber-route ownership gates, NaN-safe prices, clientlog rate-limit, deploy-secret hygiene#13
Open
Sentinel-Bluebuilder wants to merge 2 commits into
Open
fix: subscriber-route ownership gates, NaN-safe prices, clientlog rate-limit, deploy-secret hygiene#13Sentinel-Bluebuilder wants to merge 2 commits into
Sentinel-Bluebuilder wants to merge 2 commits into
Conversation
…oy secrets
Security/correctness fixes from the end-to-end platform audit.
server.js:
- add-subscriber / add-subscribers: gate both routes with assertPlanOwnership.
Both broadcast a subscribe TX paid from THIS operator's wallet, so without
an ownership check any authenticated caller could drain the operator balance
by passing a foreign planId. Also validate planId as a positive integer and
cap the bulk route at 500 addresses (each entry is a serialized broadcast).
- safeInt(): coerce chain price strings to a finite integer with a 0/null
fallback. Bare parseInt(quote_value) yielded NaN on missing/malformed values
and silently poisoned downstream prices and lease totals. Applied to every
quote_value parse and the lease totalCost.
- /api/_clientlog: add a 120/min rate limit and clamp the tag to 80 chars so a
same-origin page can't flood server logs (CSRF middleware already blocks
cross-site callers).
public/index.html:
- txFetch: throw on non-ok HTTP responses instead of returning {} as success.
A swallowed 400/500 made callers act on an empty object and fail silently;
all 18 call sites already sit in try/catch. Keplr-sign relay handled first.
- var(--muted) -> var(--text-muted) (4 Privy surfaces). --muted was never
defined, so that text rendered with no color var and was effectively invisible.
.dockerignore:
- exclude .session-key, privy-wallets.json, auto-grant.json, an.json from the
build context so runtime secrets/session keys are never baked into the image.
This was referenced Jun 21, 2026
…ivy error
Follow-up hardening folded into the audit PR:
- add-subscriber / add-subscribers: validate addresses against the full
bech32 SENT_ADDR_RE (sent1 + 38 charset chars) instead of a bare
startsWith('sent1') prefix check, which accepted malformed/overlong
strings that only fail later at broadcast.
- batch-link / batch-unlink: cap the node list at 100 per request. Each
node adds messages to a single TX; an unbounded list can exceed the
block gas limit and saturate the RPC pool.
- Privy config error: escape the interpolated error text with escapeHtml
before injecting into innerHTML, closing a reflected-XSS vector on the
error message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
End-to-end platform audit fixes. Three independent bodies of work came out of the audit; this is PR 1 of 3 (security + correctness hardening). The other two (FE optimistic-update/reconcile, server-side plan-nodes caching) follow as separate PRs.
Changes
server.js/api/plan/add-subscriberand/api/plan/add-subscribersboth broadcast a subscribe TX paid from the operator''s own wallet, but neither verified the caller owns the target plan. Any authenticated caller could drain the operator balance by passing a foreignplanId. Both routes now callassertPlanOwnership(planIdNum)before broadcasting, validateplanIdas a positive integer, and the bulk route caps the batch at 500 addresses (each entry is a serialized broadcast that pins the wallet queue).safeInt()NaN guard. Chain price fields arrive as strings and can be missing/malformed; bareparseInt(quote_value)returnedNaN, which silently poisoned downstream prices and lease totals. NewsafeInt(v, fallback)coerces to a finite integer; applied to everyquote_valueparse and the leasetotalCost./api/_clientloghardening. Added a 120/min rate limit and clamped the tag to 80 chars so a same-origin page can''t flood server logs. (The global CSRF middleware already blocks cross-site callers.)public/index.htmltxFetchno longer swallows HTTP errors. It returned{}on a 400/500, so callers acted on an empty object and failed silently. It now throws on non-ok responses (Keplr-sign relay handled first). All 18 call sites already sit intry/catch; verified no unhandled rejections.var(--muted)→var(--text-muted)on 4 Privy surfaces.--mutedwas never defined, so that text rendered with no color variable and was effectively invisible..dockerignore.session-key,privy-wallets.json,auto-grant.json,an.jsonfrom the build context so runtime secrets/session keys are never baked into the published image.Verification
node --check server.jspasses.planIdrejected;/api/_clientlogreturns ok and trips 429 after 120 req/min (confirmed against the running server).